add filters to prompt function #1371
Conversation
outlines/prompts.py
Outdated
| return env.get_template(os.path.basename(path)) | ||
|
|
||
|
|
||
| def prompt(fn: Callable) -> Prompt: | ||
| def prompt(fn: Optional[Callable] = None, **filters: Dict[str, Callable]) -> Callable: |
There was a problem hiding this comment.
filters should be a simple argument that has type Dict[str, Callable] and maps the filter names to the functions.
outlines/prompts.py
Outdated
| @@ -118,10 +125,14 @@ def _template_from_file(_, path: Path) -> jinja2.Template: | |||
| keep_trailing_newline=True, | |||
| undefined=jinja2.StrictUndefined, | |||
| ) | |||
|
|
|||
| for name, filter_fn in filters.items(): | |||
There was a problem hiding this comment.
I generally don't mind a little repetition but I think we should refactor the code and add a create_environment class that can be used in both class methods. @yvan-sraka are there any reasons we shouldn't do that?
There was a problem hiding this comment.
I agree, ideally, it would be less surprising for users to have the most similar environment (and behavior) for any method used to create a Prompt. Otherwise, it's a great feature addition!
There was a problem hiding this comment.
I added a create_jinja_env function.
|
thanks for your review @rlouf. I've taken your comments into account. While doing so the following alternative approach came to me: what do you think? |
|
Thank you for being attentive to the general design! I think your original design to be simpler, we should go with it (including my comments). |
|
ok, great! Do you have any more comments about the implementation? |
outlines/prompts.py
Outdated
| if isinstance(filters, list): | ||
| for filter_fn in filters: | ||
| env.filters[filter_fn.__name__] = filter_fn |
There was a problem hiding this comment.
Let's stick to filters being a dictionary.
There was a problem hiding this comment.
I removed the list option.
|
Looks good to me! @yvan-sraka ? |
|
Awesome! |
Allow giving custom filters to the prompt decorator